Port aligned_accessor from the Raft fork of Kokkos to mainline.#422
Port aligned_accessor from the Raft fork of Kokkos to mainline.#422vitor1001 wants to merge 2 commits intokokkos:stablefrom
aligned_accessor from the Raft fork of Kokkos to mainline.#422Conversation
See https://github.com/rapidsai/raft/blob/branch-25.08/cpp/include/raft/thirdparty/mdspan/include/experimental/__p0009_bits/aligned_accessor.hpp. This implementation was adapted from kokkos/mdspan/blob/stable/examples/aligned_accessor/aligned_accessor.cpp.
nmm0
left a comment
nmm0
left a comment
There was a problem hiding this comment.
Thank you for submitting this. This is still also missing some tests and is_sufficiently_aligned
This was on my TODO list -- if you would like, I can take this over or I can continue to review this.
| @@ -0,0 +1,783 @@ | |||
| /* | |||
There was a problem hiding this comment.
We should remove this file as we already have padded layouts
| constexpr bool | ||
| is_nonzero_power_of_two(const std::size_t x) | ||
| { | ||
| // Just checking __cpp_lib_int_pow2 isn't enough for some GCC versions. | ||
| // The <bit> header exists, but std::has_single_bit does not. | ||
| #if defined(__cpp_lib_int_pow2) && __cplusplus >= 202002L | ||
| return std::has_single_bit(x); | ||
| #else | ||
| return x != 0 && (x & (x - 1)) == 0; | ||
| #endif | ||
| } | ||
|
|
||
| template<class ElementType> | ||
| constexpr bool | ||
| valid_byte_alignment(const std::size_t byte_alignment) | ||
| { | ||
| return is_nonzero_power_of_two(byte_alignment) && byte_alignment >= alignof(ElementType); | ||
| } | ||
|
|
||
| // We define aligned_pointer_t through a struct | ||
| // so we can check whether the byte alignment is valid. | ||
| // This makes it impossible to use the alias | ||
| // with an invalid byte alignment. | ||
| template<class ElementType, std::size_t byte_alignment> | ||
| struct aligned_pointer { | ||
| static_assert(valid_byte_alignment<ElementType>(byte_alignment), | ||
| "byte_alignment must be a power of two no less than " | ||
| "the minimum required alignment of ElementType."); | ||
| using type = ElementType* _MDSPAN_ALIGN_VALUE_ATTRIBUTE( byte_alignment ); | ||
| }; |
There was a problem hiding this comment.
All of these guys should be in a detail namespace
| @@ -0,0 +1,186 @@ | |||
| /* | |||
There was a problem hiding this comment.
Just reminding ourselves we need to fix this license as it is not the correct one anymore
| template<class ElementType, std::size_t byte_alignment> | ||
| aligned_pointer_t<ElementType, byte_alignment> | ||
| bless(ElementType* ptr, std::integral_constant<std::size_t, byte_alignment> /* ba */ ) | ||
| { | ||
| return _MDSPAN_ASSUME_ALIGNED( ElementType, ptr, byte_alignment ); | ||
| } |
There was a problem hiding this comment.
| template<class ElementType, std::size_t byte_alignment> | |
| aligned_pointer_t<ElementType, byte_alignment> | |
| bless(ElementType* ptr, std::integral_constant<std::size_t, byte_alignment> /* ba */ ) | |
| { | |
| return _MDSPAN_ASSUME_ALIGNED( ElementType, ptr, byte_alignment ); | |
| } |
From my side I prefer you take this over. You know obviously much better this codebase than I do. |
Apply suggestions Co-authored-by: Nicolas Morales <nmmoral@sandia.gov>
Ok I will do, I really appreciate your contribution. |
2 participants
See https://github.com/rapidsai/raft/blob/branch-25.08/cpp/include/raft/thirdparty/mdspan/include/experimental/__p0009_bits/aligned_accessor.hpp.
This implementation was adapted from kokkos/mdspan/blob/stable/examples/aligned_accessor/aligned_accessor.cpp.
Fixes #412.